Skip to content

Add tsv mimeType support and raise clear error for xlsx in WQP - #232

Closed
thodson-usgs wants to merge 1 commit into
DOI-USGS:mainfrom
thodson-usgs:fix-wqp-tsv-mimetype
Closed

Add tsv mimeType support and raise clear error for xlsx in WQP#232
thodson-usgs wants to merge 1 commit into
DOI-USGS:mainfrom
thodson-usgs:fix-wqp-tsv-mimetype

Conversation

@thodson-usgs

@thodson-usgs thodson-usgs commented Apr 13, 2026

Copy link
Copy Markdown
Collaborator

Closes #162

Summary

  • _check_kwargs now accepts mimeType='tsv' alongside 'csv' for legacy WQP calls
  • mimeType='xlsx' raises NotImplementedError with a clear message (xlsx support is not yet implemented)
  • Any other invalid mimeType raises ValueError: Invalid mimeType. Supported options: 'csv', 'tsv'.
  • Extracted _read_wqp_response(text, kwargs) helper that replaces 9 identical pd.read_csv(..., delimiter=",") calls — uses \t delimiter when mimeType='tsv', , otherwise

Note: tsv support applies to legacy WQP calls only. WQX3.0 endpoints only support csv at this time, as noted in the original issue.

Test plan

  • test_check_kwargs updated to cover tsv (passes), xlsx (NotImplementedError), and unchanged cases
  • test_get_results_tsv added: mocks a tab-delimited response and verifies it is parsed correctly
  • All 94 unit tests pass

🤖 Generated with Claude Code

@thodson-usgs thodson-usgs mentioned this pull request May 6, 2026
Closed
@thodson-usgs @claude
feat(wqp): accept tsv mimeType and reject xlsx with a clear error
788bda1 
`_check_kwargs` now accepts `mimeType=tsv` alongside `csv`, defaults a
missing mimeType to csv, and raises a clear NotImplementedError for
`xlsx` that points at the csv/tsv options. `_read_wqp_csv` gained a
`delimiter` argument (tab for tsv, comma otherwise, selected by the new
`_wqp_delimiter` helper) so tsv responses parse correctly while still
preserving leading zeros on code columns; `get_results` and the shared
`_what` helper pass the mimeType-derived delimiter.

Re-authored onto main's `_what`/`_read_wqp_csv` structure (it predated the
DOI-USGS#320 what_* consolidation, the DOI-USGS#311 leading-zero fix, and the httpx
migration); the branch's stale `requests_mock` tests are replaced with
offline unit tests for the new behavior. Addresses DOI-USGS#162 (tsv support; xlsx
now fails with a clear, actionable message rather than silently).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Sjb14HkwuCydKSKMsaXsgd
@thodson-usgs
thodson-usgs force-pushed the fix-wqp-tsv-mimetype branch from dd00e2c to 788bda1 Compare June 25, 2026 13:57
@thodson-usgs

thodson-usgs commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator Author

Closing this after re-evaluating it against current main. This branch is 109 commits behind and every hunk it touches has since been rewritten, so landing it would be a from-scratch reimplementation rather than a merge — worth checking first whether it should exist at all. I don't think it should.

Measured against the live portal

Legacy WQP does serve all three formats, so the premise of #162 holds:

mimeType status content-type
csv 200 text/csv
tsv 200 text/tab-separated-values
xlsx 200 application/vnd.openxmlformats-officedocument.spreadsheetml.sheet

But the getters return a DataFrame, so the wire format is unobservable to callers. csv and tsv parse to identical frames:

csv: shape=(2031, 63)   tsv: shape=(2031, 63)
columns identical: True
pd.testing.assert_frame_equal(csv, tsv)  ->  passes

Worse, WQX3 silently ignores mimeType=tsv and returns a comma-delimited body anyway (Content-Type: text/plain). This branch has no legacy guard, so _wqp_delimiter() picks "\t" and pandas swallows each whole row as a single field — no exception, no warning:

legacy=False + mimeType='tsv'  ->  (2334, 1)
first column: 'Org_Identifier,Org_FormalName,Project_Identifier,Project_Name,...'

That is precisely the quiet mangling _read_wqp_csv / _is_code_column were added to prevent.

Against current conventions

  • main's _check_kwargs now says "Pass mimeType='csv', or omit it -- csv is the only format this package parses." That was written after this PR opened, under the AGENTS.md error-message policy. This PR reverses it.
  • The NotImplementedError for xlsx conflicts with AGENTS.md's "Every check raises ValueError -- one class for a bad argument value, so a caller catches by shape rather than by which module rejected it." main's current xlsx message is already clearer than the one proposed here.
  • mimeType is not a term in CONTEXT.md, and it is not a setting — "a public keyword is not automatically a setting." This would add a legacy-only, mode-dependent public promise for a knob with no observable effect.

xlsx fares no better: it would pull in openpyxl and still produce the same DataFrame.

If this comes back

The motivation to look for is malformed WQP CSV quoting breaking real parses — a caller hitting ParserError on a free-text field. That would be evidence-backed and would justify a different change (a documented escape hatch, guarded to legacy=True). No such report exists today.

Closing #162 alongside for the same reasons.

@thodson-usgs thodson-usgs mentioned this pull request Aug 26, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Accommodate tsv, xlsx mimeTypes in wqp.py

1 participant

@thodson-usgs